Add restart command and --watch option to wp shell#77
Conversation
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This PR introduces very useful restart and --watch features to wp shell. The implementation is well-structured, especially the use of a special exit code for restarting and the pcntl_exec for a full process reload. The file watching logic is also robustly implemented with recursive directory scanning and proper error handling.
I've added a couple of suggestions to improve the robustness of argument handling during the restart process. Also, while there are good tests for the restart and exit commands, consider adding tests for the new --watch functionality to prevent future regressions.
| // Add the path argument if present in $_SERVER | ||
| if ( isset( $_SERVER['argv'] ) && is_array( $_SERVER['argv'] ) ) { | ||
| foreach ( $_SERVER['argv'] as $arg ) { | ||
| if ( is_string( $arg ) && '--path=' === substr( $arg, 0, 7 ) ) { | ||
| $args[] = $arg; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation for preserving the --path argument is fragile. It only works for the --path=/some/path format and will fail if the argument is passed with a space, like --path /my/path. This can lead to the shell restarting in an incorrect directory.
A more robust approach is to retrieve the configuration value directly from WP-CLI's runner. This would also make it easier to preserve other important global parameters (e.g., --user, --url) to ensure a consistent environment after restarting.
// Add the path argument if present from the runner's config.
$config = WP_CLI::get_runner()->config;
if ( isset( $config['path'] ) ) {
$args[] = '--path=' . $config['path'];
}| $wp_cli_script = null; | ||
| foreach ( array( 'argv', '_SERVER' ) as $source ) { | ||
| if ( 'argv' === $source && is_array( $GLOBALS['argv'] ) && isset( $GLOBALS['argv'][0] ) ) { | ||
| $wp_cli_script = $GLOBALS['argv'][0]; | ||
| break; | ||
| } elseif ( '_SERVER' === $source && is_array( $_SERVER['argv'] ) && isset( $_SERVER['argv'][0] ) ) { | ||
| $wp_cli_script = $_SERVER['argv'][0]; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic to determine the WP-CLI script path is unnecessarily complex. It can be simplified for better readability and maintainability by using a direct if/elseif check on $GLOBALS['argv'] and $_SERVER['argv'] instead of a foreach loop.
$wp_cli_script = null;
if ( isset( $GLOBALS['argv'][0] ) ) {
$wp_cli_script = $GLOBALS['argv'][0];
} elseif ( isset( $_SERVER['argv'][0] ) ) {
$wp_cli_script = $_SERVER['argv'][0];
}
Implementation Complete
The
restartcommand now spawns a new PHP process usingpcntl_exec(), which fully reloads all code including files loaded withrequire_once/include_once. This addresses the original issue requirement to "properly spawn the actual REPL in a new PHP process" for reloading modified code. Ifpcntl_exec()is not available, the shell falls back to in-process restart with a debug message.All requirements from the issue have been implemented and code style issues have been addressed.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.